Skip to content
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 ptp setup parameters #110

Merged
merged 20 commits into from
Mar 8, 2024
Merged

feat: hesai ptp setup parameters #110

merged 20 commits into from
Mar 8, 2024

Conversation

amc-nu
Copy link
Contributor

@amc-nu amc-nu commented Dec 27, 2023

PR Type

  • New Feature

Description

Add support to the hesai hw interface to setup the Lidar's PTP profile.

Review Procedure

You can now set up the PTP profile using the following arguments in the XML and Python launch files.

<arg name="ptp_profile" default="1588v2" description="1588v2|802.1as|automotive"/>
<arg name="ptp_domain" default="0" description="PTP Domain [0-127]."/>
<arg name="ptp_transport_type" default="UDP" description="1588v2 supports 'UDP' or 'L2', other profiles only L2 (HW)"/>

Remarks

  • Make sure you have a PTP server running on the host PC
  • Only some of the sensors support all the available profiles. Please verify the respective manuals to the corresponding model you are testing with.

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.

@amc-nu amc-nu requested a review from drwnz December 27, 2023 09:15
@amc-nu amc-nu changed the title Hesai ptp params feature. Hesai ptp params Dec 27, 2023
@amc-nu amc-nu changed the title feature. Hesai ptp params feat: Hesai ptp params Dec 27, 2023
@codecov-commenter
Copy link

codecov-commenter commented Dec 27, 2023

Codecov Report

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

Project coverage is 8.35%. Comparing base (910d12b) to head (1d59276).

Files Patch % Lines
nebula_ros/src/hesai/hesai_decoder_ros_wrapper.cpp 0.00% 55 Missing ⚠️
...a_ros/src/hesai/hesai_hw_interface_ros_wrapper.cpp 0.00% 33 Missing ⚠️
.../nebula_hesai_hw_interfaces/hesai_hw_interface.cpp 0.00% 31 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##            main    #110      +/-   ##
========================================
+ Coverage   6.58%   8.35%   +1.76%     
========================================
  Files        136      67      -69     
  Lines      10987    8232    -2755     
  Branches     869     859      -10     
========================================
- Hits         724     688      -36     
+ Misses      9677    6960    -2717     
+ Partials     586     584       -2     
Flag Coverage Δ
differential 8.35% <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.

@amc-nu amc-nu changed the title feat: Hesai ptp params feat: hesai ptp setup parameters Dec 27, 2023
@drwnz
Copy link
Collaborator

drwnz commented Feb 27, 2024

I tested PTP operation and the xml launchfiles. The Python launchfile does not appear to work. It does not set the rotation speed, which has a default as a non-integer causing a launch error. I will submit a PR for the rotation error issue, but the python launcher should probably access more setting (like rotation speed, destination IP).

Copy link
Collaborator

@drwnz drwnz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@drwnz drwnz merged commit 45c01cf into main Mar 8, 2024
7 checks passed
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.

3 participants