Skip to content

refactor(nebula_hw_interfaces): use nebula logger everywhere#305

Merged
mojomex merged 5 commits intomainfrom
refactor/use-nebula-logger-everywhere
Apr 11, 2025
Merged

refactor(nebula_hw_interfaces): use nebula logger everywhere#305
mojomex merged 5 commits intomainfrom
refactor/use-nebula-logger-everywhere

Conversation

@mojomex
Copy link
Collaborator

@mojomex mojomex commented Apr 11, 2025

PR Type

  • Improvement

Related Links

Description

Replace all RCLCPP_ and std::cout, std::cerr etc. usage by logger-> calls for all remaining vendors. For Hesai, this has already been done in #232, but there were some print statements remaining.

Supersedes #301.

Review Procedure

  • code review (should be straight-forward), commits are split by sensor/vendor
  • confirm using PCAP or sensor that logging works as expected

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.

mojomex added 5 commits April 11, 2025 13:39
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
Signed-off-by: Max SCHMELLER <max.schmeller@tier4.jp>
@mojomex mojomex requested a review from knzo25 April 11, 2025 04:54
@mojomex mojomex changed the title Refactor/use nebula logger everywhere refactor(nebula_hw_interfaces): use nebula logger everywhere Apr 11, 2025
Copy link
Contributor

@knzo25 knzo25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (can you please check the comments I made just in case?)

@mojomex
Copy link
Collaborator Author

mojomex commented Apr 11, 2025

@knzo25 Damn, you're fast 👀
I addressed the comments, I'll merge once the conversations are resolved.

@mojomex mojomex self-assigned this Apr 11, 2025
@codecov
Copy link

codecov bot commented Apr 11, 2025

Codecov Report

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

Project coverage is 31.41%. Comparing base (c20d385) to head (49e5669).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #305      +/-   ##
==========================================
- Coverage   32.70%   31.41%   -1.30%     
==========================================
  Files         112      112              
  Lines        9750     9610     -140     
  Branches     5017     4089     -928     
==========================================
- Hits         3189     3019     -170     
- Misses       5967     6040      +73     
+ Partials      594      551      -43     
Flag Coverage Δ
differential 31.41% <0.00%> (?)
total ?

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

Components Coverage Δ
Common 42.00% <ø> (-12.36%) ⬇️
Hesai 29.22% <0.00%> (-1.05%) ⬇️
Velodyne 40.04% <0.00%> (-0.25%) ⬇️
Continental 31.02% <0.00%> (+0.17%) ⬆️
Robosense 5.64% <0.00%> (-0.05%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@mojomex mojomex merged commit 54b5746 into main Apr 11, 2025
11 of 19 checks passed
@mojomex mojomex deleted the refactor/use-nebula-logger-everywhere branch April 11, 2025 06:02
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