generated from tier4/ros2-project-template
-
Notifications
You must be signed in to change notification settings - Fork 58
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
feat(hesai): multicast support #187
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knzo25
reviewed
Sep 4, 2024
...hw_interfaces/include/nebula_hw_interfaces/nebula_hw_interfaces_hesai/hesai_cmd_response.hpp
Outdated
Show resolved
Hide resolved
knzo25
reviewed
Sep 4, 2024
nebula_ros/include/nebula_ros/continental/continental_ars548_hw_interface_wrapper.hpp
Outdated
Show resolved
Hide resolved
mojomex
force-pushed
the
hesai-multicast
branch
from
September 5, 2024 07:42
5f63f5e
to
529c9bd
Compare
knzo25
reviewed
Sep 6, 2024
knzo25
reviewed
Sep 6, 2024
nebula_ros/include/nebula_ros/continental/continental_ars548_hw_interface_wrapper.hpp
Outdated
Show resolved
Hide resolved
FYI:
|
Ah, indeed that's annoying. For now, as GCC is the standard, I will prioritize having no warnings when compiling with it instead of Clang. |
knzo25
approved these changes
Sep 7, 2024
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Type
Related Links
Description
Hesai sensors do not officially mention multicast support in their datasheets, but it turns out that it works just fine. Tested sensors:
This PR adds a
multicast_ip
parameter for all Hesai sensors. Nebula sets the destination IP of the sensor to that multicast IP whensetup_sensor
is enabled andmulticast_ip
is not an empty string.If
setup_sensor
is enabled andmulticast_ip
is empty, thehost_ip
is set as the destination IP instead, in the same way it was being done up until now.Review Procedure
multicast_ip
to an empty string. Nebula should behave as usual.multicast_ip
to an address in the range224.0.0.0/4
and enablesetup_sensor
. The sensor data should still arrive and the hardware monitor should still work.Remarks
To confirm the sensor really supports multicast, check the destination MAC address of the arriving frames. The last three bytes should equal the last three bytes of the IP multicast address, and Wireshark should show IPv4mcast_xx:xx:xx as the L2 destination name:
Pre-Review Checklist for the PR Author
PR Author should check the checkboxes below when creating the PR.
Checklist for the PR Reviewer
Reviewers should check the checkboxes below before approval.
Post-Review Checklist for the PR Author
PR Author should check the checkboxes below before merging.
CI Checks