feat(hesai): add support for xt16

[jemmmel]

PR Type

• New Feature

Related Links

PCAP data

Description

Added support for XT16 LiDAR, based on existing XT32

• Added new header files and config
• Modified existing code with XT16 references
• Added test bag file and .pcd

Remarks

Related to #137

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.

[mojomex]

Evaluation

Code Review

Everything looks really god, thank you! I left some small suggestions, XT16 was missing in two switch statements.

Unit Tests

Decoder tests are well formed and passing 👍

PCAP Replay

Using a currently closed-source tool, I instantiated a dummy XT16 using the information in the PCAP. Nebula connects without a problem and pointcloud and diagnostics output work without a problem (see screenshot)

It seems that, just like XT32, tthe power diagnostics have garbage values in them. Since we don't have the newest TCP protocol specification, this is curently a won't fix in Nebula, so as far as your PR is concerned, it's ok to leave as-is 🙆‍♂️

Documentation

Could you add XT16 to the list of supported sensors, and add a parameter page for the model?

My apologies, the develop branch was a bit behind main, so not all doc changes were reflected there. Could you rebase your PR onto the updated develop? 🙇

[jemmmel]

All review comments done, submitting for review

[mojomex]

@jemmmel Looks good, thanks!
Could you rebase onto develop once again? Since it was updated when your PR already existed, there is a lot of now-duplicated history in your branch 🙇

[jemmmel]

@mojomex Yeah no worries, I think my method was bad. Do you know how to do this without pulling in the unneeded history?
Previously I tried

1. Update develop branch with tier4 changes
2. Switch to pr branch and git rebase develop

[mojomex]

@jemmmel Rebasing something with merge commits inbetween is quite painful, so the easiest method is to

git fetch --all
git reset --soft upstream/develop

and then re-create the commits by hand. It doesn't have to be the exact same commit history, just roughly divided into the different steps (add parameter file, add to nebula_common etc, add tests).

The rebase you tried just now lost a few changes, e.g. in hesai_common.

[jemmmel]

Hey @mojomex, thanks for the tips. Since the changes were minor and I was making a mess of the rebase, I have done some dectructive pushing, sorry about that. I also had an issue with signing commits. Anyway I think I have fixed it now, sorry about that

[mojomex]

LGTM except for this missing import 🙇

[mojomex]

LGTM, will merge when CI passes 🎉

Thanks for your contribution!

[jemmmel]

Great, glad to be a part of it! Thanks for your help as well @mojomex

[jemmmel]

Added missing vendor file to fix the JSON schema check failing

[mojomex]

@jemmmel Could you do

diff nebula_ros/schema/PandarXT16.schema.json nebula_ros/schema/PandarXT32.schema.json

And migrate over the slight differences from XT32? Sensor model enum value and some references to other schemas changed. Then schema check should pass 🙂

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 10 lines in your changes missing coverage. Please review.

Project coverage is 26.23%. Comparing base (3d42cd0) to head (3c8c9ff).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ers/nebula_decoders_hesai/decoders/pandar_xt16.hpp	75.00%	2 Missing and 1 partial ⚠️
...ommon/include/nebula_common/hesai/hesai_common.hpp	0.00%	2 Missing ⚠️
...ula_common/include/nebula_common/nebula_common.hpp	60.00%	1 Missing and 1 partial ⚠️
.../nebula_hesai_hw_interfaces/hesai_hw_interface.cpp	0.00%	2 Missing ⚠️
.../nebula_hw_interfaces_hesai/hesai_cmd_response.hpp	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #241      +/-   ##
===========================================
+ Coverage    26.11%   26.23%   +0.11%     
===========================================
  Files          101      102       +1     
  Lines         9212     9233      +21     
  Branches      2211     2220       +9     
===========================================
+ Hits          2406     2422      +16     
- Misses        6417     6420       +3     
- Partials       389      391       +2     

Flag	Coverage Δ
differential	26.23% <60.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.

[jemmmel]

@jemmmel Could you do

diff nebula_ros/schema/PandarXT16.schema.json nebula_ros/schema/PandarXT32.schema.json

And migrate over the slight differences from XT32? Sensor model enum value and some references to other schemas changed. Then schema check should pass 🙂

Done now, missed some changes from the rebase!