-
Notifications
You must be signed in to change notification settings - Fork 251
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
Add --log-level to ros2 bag play and record #1625
Conversation
@MichaelOrlov could you please take a look? this is draft PR to validate design. If design is OK, I'll add tests and also cover recorder |
@r7vme Thanks for the PR! The approach looks good to me. 👍 |
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.
lgtm with a couple of minor refactoring ideas.
0886fca
to
4151c6e
Compare
@MichaelOrlov @fujitatomoya thanks for review. Addressed comments and added test, ready for review |
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.
lgtm with green CI, @r7vme thanks for iterating!
@r7vme can you rebase this there are several commits ahead in mainline rolling? and then i can start the CI. |
Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com>
4151c6e
to
df85a43
Compare
@fujitatomoya rebased |
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.
LGTM. Thanks!
@r7vme test failures are pretty much unrelated i think, but just in case can you also take a look? |
@fujitatomoya No tests related to rosbag failed as far as I can see. Most failures for aarch64 are |
@MichaelOrlov i will leave this to you, since i do not have merge permission for this repo. |
@r7vme @fujitatomoya We have a breakage on CI currently. Will wait until ros2/ros2#1560 (review) will be merged - then will re-run CI one more time. |
Pulls: #1625 |
@r7vme Could you please address one new warning https://ci.ros2.org/job/ci_windows/21723/msbuild/new/ on Windows CI build? |
Signed-off-by: Michael Orlov <michael.orlov@apex.ai>
@ros-pull-request-builder retest this please |
@r7vme Could you please help to backport this PR to the Iron and Humble? |
|
https://github.com/Mergifyio backport jazzy |
✅ Backports have been created
|
* Add log level parameter to rosbag play and record Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> * Fix warning with typecast and add test coverage for default constructors Signed-off-by: Michael Orlov <michael.orlov@apex.ai> --------- Signed-off-by: Roman Sokolkov <rsokolkov@gmail.com> Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Michael Orlov <michael.orlov@apex.ai> (cherry picked from commit 5a06430)
* Add log level parameter to rosbag play and record * Fix warning with typecast and add test coverage for default constructors --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Roman Sokolkov <rsokolkov@gmail.com> (cherry picked from commit 5a06430)
* Add log level parameter to rosbag play and record * Fix warning with typecast and add test coverage for default constructors --------- Signed-off-by: Michael Orlov <michael.orlov@apex.ai> Co-authored-by: Roman Sokolkov <rsokolkov@gmail.com> (cherry picked from commit 5a06430) Co-authored-by: Roman <rsokolkov@gmail.com>
--log-level
toros2 bag play
to allow print e.g. debug logs. Currently user can not change default logging level (info).Examples:
info
debug