feat: allow to launch hardware in listen mode

[msz-rai]

PR Type

• Improvement

Related Links

#136

Description

This PR introduces a new parameter to the hesai_ros_wrapper and velodyne_ros_wrapper nodes, allowing hardware to launch with communication to the sensor disabled. In this configuration, the driver operates in a "listen-only" mode, receiving packet data without sending commands.

This functionality is required for the AWSIM simulator, where the LiDAR simulation plugin supports publishing raw packets via UDP but doesn't handle communication from the driver.

Prior to nebula version 0.2.0, there was a workaround for this behaviour (as outlined in the related issue). However, recent architectural changes have removed the ability to use that workaround (segmentation fault appears).

Review Procedure

Code review + test to ensure that the new parameter does not interfere with the normal operation of real-world sensors.

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.

[codecov]

Codecov Report

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

Project coverage is 0.00%. Comparing base (668c8f7) to head (c2f758e).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
nebula_ros/src/hesai/hesai_ros_wrapper.cpp	0.00%	9 Missing ⚠️
nebula_ros/src/velodyne/velodyne_ros_wrapper.cpp	0.00%	6 Missing ⚠️
nebula_ros/src/hesai/hw_interface_wrapper.cpp	0.00%	5 Missing ⚠️
nebula_ros/src/velodyne/hw_interface_wrapper.cpp	0.00%	5 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (668c8f7) and HEAD (c2f758e). Click for more details.

HEAD has 19 uploads less than BASE

Flag	BASE (668c8f7)	HEAD (c2f758e)
total	19	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #210       +/-   ##
==========================================
- Coverage   25.91%   0.00%   -25.92%     
==========================================
  Files          99      86       -13     
  Lines        9179    7845     -1334     
  Branches     2205     937     -1268     
==========================================
- Hits         2379       0     -2379     
- Misses       6415    7845     +1430     
+ Partials      385       0      -385     

Flag	Coverage Δ
differential	0.00% <0.00%> (?)
total	?

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mojomex]

Thank you for your great work!

The json-schema-check is currently failing, could you add the new parameter ton the affected sensor model and vendor schemas in nebula_ros/schema?

As for the parameter name, I think that communicate_with_sensor can easily be confused with launch_hw, as in, users could think that this affects ALL communication including UDP.
Imho, a more fitting name would be disable_tcp or udp_only.

Please also add a info-level log message in the wrappers that is printed when TCP is disabled, noting that checking and synchronizing settings, and diagnostics publishing are disabled.

Otherwise everything looks good to go!

[msz-rai]

@mojomex

Thank you for the review!

I agree that my parameter name suggestion was misleading. I chose udp_only instead of disable_tcp because Velodyne uses HTTP for communication.

I also applied the rest of review suggestions. PR is ready for re-review.

[mojomex]

@msz-rai Thanks for your quick response,
I'll re-review it tomorrow!

[mojomex]

LGTM!

Before we merge, we had an internal discussion because there's now three connection-related parameters (udp_only, launch_hw and setup_sensor). Having three booleans (= 8 possible combinations) for only four valid parameter combinations (launch_hw && setup_sensor && !udp_only, launch_hw && !setup_sensor && !udp_only, launch_hw && !setup_sensor && udp_only, !launch_hw) will probably be confusing for users.

We'd like to replace the three parameters with one parameter, connection_mode, which is one of offline, udp_only, skip_setup, full, for clarity.

Since this is out of scope for this PR, I'll make that change in another PR immediately after this one is merged. @msz-rai do you have any objections to this?

[msz-rai]

Everything looks good to me!

[mojomex]

I'll test with a sensor tomorrow and then merge 🙇

No HW test performed yet.

[mojomex]

👍