Skip to content

chore: changed cerrs to loggers and forced flush to couts#301

Closed
knzo25 wants to merge 1 commit intotier4:mainfrom
knzo25:chore/logger_prints
Closed

chore: changed cerrs to loggers and forced flush to couts#301
knzo25 wants to merge 1 commit intotier4:mainfrom
knzo25:chore/logger_prints

Conversation

@knzo25
Copy link
Contributor

@knzo25 knzo25 commented Apr 4, 2025

PR Type

  • Improvement

Related Links

Description

Some error were being printed as cerr which in some cases is not captured by the logging system.
Additionally, in ROS, stoud and stderr is sometimed not flushed so manual flushes are required.
Since we do not excessively flush, it should not be a problem

Review Procedure

Remarks

Pre-Review Checklist for the PR Author

PR Author should check the checkboxes below when creating the PR.

  • Assign PR to reviewer

Checklist for the PR Reviewer

Reviewers should check the checkboxes below before approval.

  • Commits are properly organized and messages are according to the guideline
  • (Optional) Unit tests have been written for new behavior
  • PR title describes the changes

Post-Review Checklist for the PR Author

PR Author should check the checkboxes below before merging.

  • All open points are addressed and tracked via issues or tickets

CI Checks

  • Build and test for PR: Required to pass before the merge.

Signed-off-by: Kenzo Lobos-Tsunekawa <kenzo.lobos@tier4.jp>
@knzo25 knzo25 requested a review from mojomex April 4, 2025 03:59
@codecov
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 32.29%. Comparing base (d555138) to head (702ae97).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #301      +/-   ##
==========================================
- Coverage   32.70%   32.29%   -0.42%     
==========================================
  Files         112      112              
  Lines        9750     9750              
  Branches     5016     4978      -38     
==========================================
- Hits         3189     3149      -40     
- Misses       5967     6020      +53     
+ Partials      594      581      -13     
Flag Coverage Δ
differential 32.29% <0.00%> (?)
total ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Common 50.30% <ø> (-4.06%) ⬇️
Hesai 30.24% <0.00%> (-0.03%) ⬇️
Velodyne 40.24% <0.00%> (-0.04%) ⬇️
Continental 30.80% <0.00%> (-0.05%) ⬇️
Robosense 5.66% <0.00%> (-0.03%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mojomex
Copy link
Collaborator

mojomex commented Apr 11, 2025

Since PR #233, Nebula provides a built-in way to ensure that ROS logging functions are used when launched using ROS.
This logger has already been integrated for the Hesai HW interface in #232, and I took your PR as an opportunity of doing it for every remaining vendor: #305.

Since it is the more general approach (and was long overdue), would you mind closing this PR in favor of #305? 🙇

@knzo25 knzo25 closed this Apr 11, 2025
@mojomex
Copy link
Collaborator

mojomex commented Apr 11, 2025

Thank you for your quick response!

@mojomex
Copy link
Collaborator

mojomex commented Apr 11, 2025

Closed in favor of #305.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants