-
Notifications
You must be signed in to change notification settings - Fork 55
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
fix(hesai): set PTP lock offset on sensor configuration #239
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #239 +/- ##
==========================================
- Coverage 26.11% 26.07% -0.05%
==========================================
Files 101 101
Lines 9213 9232 +19
Branches 2212 2213 +1
==========================================
+ Hits 2406 2407 +1
- Misses 6418 6436 +18
Partials 389 389
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Evaluation
Here is a demo of OT128: ptp-lock-offset.mp4It has been confirmed that by restarting Nebula, for OT128, the setting can revert to the old value (Hesai firmware bug or PTC documentation inaccurate). With this PR, it is immediately set to the desired value again. For the other sensors tested that support th command, this bug was not observed. The value persisted over Nebula restarts. |
6fc9002
to
d3c6bf4
Compare
Restricting this feature to only affect OT128 and QT128, as other sensors have been found to not support it (maybe a firmware update would change that). Sadly, the current architecture for sensor configurations in Nebula forces us to add the parameter to ALL Hesai sensors, even if without any effect. |
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.
Please add to the documentation as well, but otherwise LGTM!
Signed-off-by: Max SCHMELLER <[email protected]>
Signed-off-by: Max SCHMELLER <[email protected]>
…d ROS2 wrapper Signed-off-by: Max SCHMELLER <[email protected]>
Signed-off-by: Max SCHMELLER <[email protected]>
… AT128 Signed-off-by: Max SCHMELLER <[email protected]>
Signed-off-by: Max SCHMELLER <[email protected]>
d3c6bf4
to
3007f2d
Compare
Signed-off-by: Max SCHMELLER <[email protected]>
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.
Please add to the documentation as well, but otherwise LGTM!
If this is done, LGTM.
PR Type
Related Links
Description
This PR adds support for the
SET_PTP_LOCK_OFFSET
andGET_PTP_LOCK_OFFSET
PTC commands.In the inquiry linked above, it was observed that not explicitly setting PTP lock offset sometimes causes the value to reset to
1 us
on at least OT128. While this is likely a sensor firmware bug, being able to set this offset from within Nebula should improve usability, and should eliminate the need to use Hesai's web interface or desktop GUI.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