IsaacLab icon indicating copy to clipboard operation
IsaacLab copied to clipboard
verified publisher icon isaac-sim

Adds RTXLidar

Open jtigue-bdai opened this issue 8 months ago • 3 comments

Description

This PR creates a sensor based on the RTX Lidar:

Fixes #https://github.com/isaac-sim/IsaacLab/issues/865

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • [ ] I have run the pre-commit checks with ./isaaclab.sh --format
  • [ ] I have made corresponding changes to the documentation
  • [ ] My changes generate no new warnings
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • [ ] I have added my name to the CONTRIBUTORS.md or my name already exists there

jtigue-bdai avatar Apr 14 '25 20:04 jtigue-bdai

Hi @jtigue-bdai, thanks for the great work on this!

I pulled in your branch to work on adding MDP support for the RTX Lidar. Had two questions:

  1. Is there a better name for the data field in rtx_lidar_data.py? The comment was useful but was not initially intuitive. I assume it's referencing this annotator documentation. Maybe it should be called point_cloud?
  2. I can add this this PR or wait until this is merged, am new to open source so not sure what best practice is here haha

Thanks!

samirchowd avatar Apr 15 '25 02:04 samirchowd

Hey @samirchowd thanks for that suggestion. "sensor.data.output['data']" is gonna look pretty weird. Yeah I think point_cloud is a good choice.

I can make the point cloud change. As for the mdp terms I'm not sure you can make a pr to this branch since I moved it out of the IsaacSim/isaaclab into a private fork. If you can go for it.

Depending on how much you were thinking of adding, one thing I could do is make an empty diff in the mdp files you were planning to work on and then you can add them to this as a suggestion and I can add you to the contributors list.

If there are a lot of additions then it would be easier as a PR separately.

jtigue-bdai avatar Apr 15 '25 10:04 jtigue-bdai

To limit the scope of the PR, I can raise it in a separate PR once this is merged in that case. I think it'll keep your development and testing simpler.

samirchowd avatar Apr 15 '25 17:04 samirchowd

Hey @jtigue-bdai, any chance of this getting reviewed soon? Would be more than happy to help out any way 😄 Thanks!

samirchowd avatar May 14 '25 16:05 samirchowd