generated from tier4/ros2-project-template
-
Notifications
You must be signed in to change notification settings - Fork 51
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_hw_interface): configure and check SO_RCVBUF
automatically
#186
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
5 tasks
knzo25
reviewed
Sep 3, 2024
nebula_ros/include/nebula_ros/continental/continental_ars548_hw_interface_wrapper.hpp
Outdated
Show resolved
Hide resolved
knzo25
reviewed
Sep 3, 2024
...hw_interfaces/include/nebula_hw_interfaces/nebula_hw_interfaces_hesai/hesai_cmd_response.hpp
Outdated
Show resolved
Hide resolved
knzo25
requested changes
Sep 3, 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.
Left some small questions 🙏
mojomex
force-pushed
the
hesai-auto-kernel-buffer-size
branch
from
September 5, 2024 07:46
d865579
to
e888255
Compare
knzo25
approved these changes
Sep 5, 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
5 tasks
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
High-bandwidth sensors such as the OT128 may experience packet loss due to the UDP socket's receive buffer being too small by default. This PR adds functionality to set and confirm that buffer size.
The buffer has to be set to a size that
For now, the chosen (hard-coded) size is
1500 B * 3600 = 5.4 MB
, which is the MTU size (1500 B
) times the maximum number of packets in one full scan of OT128.The memory footprint is still 'kind of' small compared to the size of pointcloud buffers etc., so condition 1 is fulfilled for the moment. In the future, computing the optimal buffer size based on sensor bandwidth would be preferable.
Review Procedure
Setting
rmem_default
andrmem_max
is done like:sudo sysctl -w net.core.rmem_default=123
.rmem_default
andrmem_max
to a size smaller than5.4 MB
, e.g.500000
for500 KB
. Nebula should refuse to launch with an error message telling you to increasermem_max
.rmem_default
to a small size like above, and setrmem_max
to a value greater than5.4 MB
, e.g.1000000000
for1 GB
. Nebula should now run, set its socket's buffer size to5.4 MB
and there should be no packet loss.For the easiest review experience, comment out all point filters in
hesai_decoder.hpp
and hard-code the points' distance to be a fixed value. That way, you will see a perfect circle and immediately notice any missing secitons.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