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

fix(hesai_hw_interface): hesai interface #72

Merged
merged 8 commits into from
Oct 25, 2023
Merged

Conversation

amc-nu
Copy link
Contributor

@amc-nu amc-nu commented Sep 11, 2023

PR Type

  • Improvement

Related Links

https://tier4.atlassian.net/browse/RT1-3200
https://tier4.atlassian.net/browse/RT1-3111

Description

Improves reliability of Hesai HW Interface.:

  • Allows sensor_setup:=False
  • Removes crashing when sensor not available or timeout.

Includes the following features:

  • Gets LidarCalibration from sensor
  • Skips GetInventory if unable to connect.
  • Fixes Hesai monitor.
  • Check calibration file format AT128.
  • Saves calibration data from sensor.
  • Fallback to offline mode AT128
  • Checks Return Mode AT128

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.

* Fix(GetLidarCalibration)

* Skip GetInventory if unable to connect

* fix monitor

* clean

* remove comments

Signed-off-by: amc-nu <[email protected]>

* at128. remove warnings

Signed-off-by: amc-nu <[email protected]>

* decoder. check calibration

Signed-off-by: amc-nu <[email protected]>

* Saving calibration data from sensor (except AT128)

* Saving correction data from sensor (AT128)

* fix(AT128 path)

* info for debugging

* nebula_hesai_ros. at128 fallback to offline

Signed-off-by: amc-nu <[email protected]>

* syncGetLidarCalibrationFromSensor -> GetLidarCalibrationFromSensor (AT128)

* hesai_decoder_ros. message on tcp fail

Signed-off-by: amc-nu <[email protected]>

* comment out da902901c4cd900bfbf906caf76ae143fe4a88a6

* Check "start of packet" in correction data

* hesai_decoder. save dat file

Signed-off-by: amc-nu <[email protected]>

* TcpDriver startup delay for setup_sensor

* delay & retry

* at1282ex. return mode checks

Signed-off-by: amc-nu <[email protected]>

* check lock

* check "isOpen"

---------

Signed-off-by: amc-nu <[email protected]>
Co-authored-by: Kyutoku <[email protected]>
@amc-nu amc-nu changed the title improvement: Hesai HW interface fix: Hesai HW interface Sep 11, 2023
Signed-off-by: amc-nu <[email protected]>
Signed-off-by: amc-nu <[email protected]>
@amc-nu amc-nu changed the title fix: Hesai HW interface fix(hesai_hw_interface): hesai interface Sep 11, 2023
@amc-nu amc-nu requested a review from drwnz September 14, 2023 01:55
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2023

Codecov Report

Attention: 339 lines in your changes are missing coverage. Please review.

Comparison is base (74cdda4) 9.93% compared to head (a6879f7) 9.52%.
Report is 1 commits behind head on main.

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

Additional details and impacted files
@@           Coverage Diff            @@
##            main     #72      +/-   ##
========================================
- Coverage   9.93%   9.52%   -0.41%     
========================================
  Files        118      51      -67     
  Lines      10149    6938    -3211     
  Branches    1533     828     -705     
========================================
- Hits        1008     661     -347     
+ Misses      8036    5719    -2317     
+ Partials    1105     558     -547     
Flag Coverage Δ
differential 9.52% <7.88%> (?)
total ?

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

Files Coverage Δ
.../nebula_hw_interfaces_hesai/hesai_cmd_response.hpp 0.00% <ø> (ø)
...ebula_ros/hesai/hesai_hw_interface_ros_wrapper.hpp 0.00% <ø> (ø)
.../nebula_ros/hesai/hesai_hw_monitor_ros_wrapper.hpp 0.00% <ø> (ø)
...ula_ros/src/hesai/hesai_hw_monitor_ros_wrapper.cpp 0.00% <0.00%> (ø)
nebula_tests/hesai/hesai_common.hpp 14.81% <0.00%> (-25.19%) ⬇️
nebula_tests/hesai/hesai_ros_decoder_test_main.cpp 34.09% <34.09%> (ø)
nebula_tests/hesai/hesai_ros_decoder_test.cpp 35.02% <25.00%> (ø)
...a_ros/src/hesai/hesai_hw_interface_ros_wrapper.cpp 0.00% <0.00%> (ø)
nebula_ros/src/hesai/hesai_decoder_ros_wrapper.cpp 0.00% <0.00%> (ø)
.../nebula_hesai_hw_interfaces/hesai_hw_interface.cpp 0.00% <0.00%> (ø)

... and 57 files with indirect coverage changes

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

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.

When restarting the driver multiple times with the device connected, after the 4th attempt I get this warning:
[component_container-1] [WARN] [1697789855.726097568] [hesai_hw_monitor_ros_wrapper_node]: diagnostic_updater: No HW_ID was set. This is probably a bug. Please report it. For devices that do not have a HW_ID, set this value to 'none'. This warning only occurs once all diagnostics are OK. It is okay to wait until the device is open before calling setHardwareID.

This is coming from diagnostic updater - I guess this is related to the TCP connection limitation we discussed? FYI this is on the AT128.
The device still works, and the configuration also seems to stick so not a major issue.

@amc-nu
Copy link
Contributor Author

amc-nu commented Oct 20, 2023

Yes, exactly. The sensor stops receiving commands.

@drwnz
Copy link
Collaborator

drwnz commented Oct 20, 2023

Yes, exactly. The sensor stops receiving commands.

It does seem to still correctly configure the sensor though?

@amc-nu
Copy link
Contributor Author

amc-nu commented Oct 20, 2023

It configures the sensor only during the first attempts. Afterwards the sensor rejects all the connections.

@drwnz
Copy link
Collaborator

drwnz commented Oct 25, 2023

I will merge this PR and we can work on the TCP rejection issue in a different PR.
Thank you for the great work, DAT file download works well.

@drwnz drwnz merged commit 476224d into main Oct 25, 2023
7 checks passed
@amc-nu amc-nu deleted the hesai-hw-improvements branch January 17, 2024 05:39
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